Fix metrics - #372
Conversation
Summary of ChangesHello @lukehinds, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request overhauls the application's telemetry system, making it more robust, debuggable, and informative. It introduces a new method for generating and persisting anonymous user IDs, enhances the data collected for various operational events, and provides a CLI-driven debug option for metrics. These changes aim to improve the reliability and utility of usage analytics while maintaining user privacy. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the metrics system by refactoring how user identity is handled, making it more robust and privacy-conscious. It also enriches the telemetry data by adding more context to the tracked events. However, I've found a critical issue in deepfabric/generator.py within the failure summarization logic that will cause a runtime error. My review focuses on fixing this bug.
| for _category, failures in self.failure_analysis.items(): | ||
| if failures: | ||
| # Get up to 3 examples for each category | ||
| examples = failures[:3] | ||
| summary["failure_examples"][category] = [ | ||
| summary["failure_examples"].append( | ||
| ( | ||
| str(ex)[:200] + "..." | ||
| if len(str(ex)) > 200 # noqa: PLR2004 | ||
| else str(ex) # noqa: PLR2004 | ||
| ) # noqa: PLR2004 | ||
| else str(ex) | ||
| ) | ||
| for ex in examples | ||
| ] | ||
| ) |
There was a problem hiding this comment.
There's a bug in this logic that will cause a runtime error. The summary["failure_examples"] variable is initialized as a dictionary, but this code attempts to call .append() on it, which is a list method. This will raise an AttributeError.
Additionally, the loop variable category was renamed to _category and is now unused, but it's required to correctly associate failure examples with their category in the dictionary. The original logic of assigning a list of examples to a dictionary key was correct.
I've provided a suggestion to fix this by restoring the correct dictionary assignment logic.
| for _category, failures in self.failure_analysis.items(): | |
| if failures: | |
| # Get up to 3 examples for each category | |
| examples = failures[:3] | |
| summary["failure_examples"][category] = [ | |
| summary["failure_examples"].append( | |
| ( | |
| str(ex)[:200] + "..." | |
| if len(str(ex)) > 200 # noqa: PLR2004 | |
| else str(ex) # noqa: PLR2004 | |
| ) # noqa: PLR2004 | |
| else str(ex) | |
| ) | |
| for ex in examples | |
| ] | |
| ) | |
| for category, failures in self.failure_analysis.items(): | |
| if failures: | |
| # Get up to 3 examples for each category | |
| examples = failures[:3] | |
| summary["failure_examples"][category] = [ | |
| ( | |
| str(ex)[:200] + "..." | |
| if len(str(ex)) > 200 | |
| else str(ex) | |
| ) | |
| for ex in examples | |
| ] |
1 participant
No description provided.